-
Notifications
You must be signed in to change notification settings - Fork 342
[FileSystem] Add openFileWithDefaultApplication method #5798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FileSystem] Add openFileWithDefaultApplication method #5798
Conversation
| success = true; | ||
| #elif defined(__APPLE__) | ||
| const std::string command = "open \"" + filename + "\""; | ||
| if (std::system(command.c_str()) == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::system() is not safe ; as a user could call this function with an input crafted to do malicious things...
So the solution would be to sanitize the inputs or the best solution is to use fork() and execvp() (for linux at least.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, sorry I didn't know it was bad practice to use std::system. I'll make the changes for both macOS and linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have replaced std::system with posix_spawn for Linux and popen for MacOS.
I haven't tested the MacOS implementation.
| success = true; | ||
| #elif defined(__APPLE__) | ||
| const std::string command = "open \"" + filename + "\""; | ||
| FILE* pipe = popen(command.c_str(), "r+"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, popen does more or less the same as std::system so still unsecure if filename is badly formatted.
Claude (🫢) tells me that posix_spawn is doable on macOS as well so
pid_t pid;
char* argv[] = {
const_cast<char*>("open"),
const_cast<char*>(filename.c_str()),
nullptr
};
int status = posix_spawn(&pid, "/usr/bin/open", nullptr, nullptr, argv, environ);
if (status == 0) {
waitpid(pid, &status, 0);
success = ( WIFEXITED(status) && WEXITSTATUS(status) == 0);
}
(similar to your linux version)
should be OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I made the changes.
|
This new API is not used (yet) in SOFA / SOFAGLFW (could be possibly added further to sofa-framework/SofaGLFW#259). Thanks for the work @EulalieCoevoet it reminds me that we should plan the convergence SofaGLFW and Compliance GUI 😉 (after Prefabs ?) |
Used to open the file where a node or component is implemented or instantiate (see SofaComplianceRobotics/SofaGLFW#92)
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if